Add egress loss stat #195
Add egress loss stat #195
Conversation
k0nserv
force-pushed
the
feature/egress-loss-stat
branch
from
5774919
to
0af7974
Compare
src/rtp/rtcp/twcc.rs
Outdated
| @@ -1088,6 +1088,7 @@ impl TwccSendRegister { | |||
| ); | |||
| } | |||
|
|
|||
| println!("TWCC loss: {:?}", self.loss(Duration::from_secs(1), now)); | |||
There was a problem hiding this comment.
Soz, left from debugging
|
|
||
| Some((lost as f64) / (total as f64)) | ||
| } | ||
|
|
There was a problem hiding this comment.
This looks a bit expensive. Is it?
There was a problem hiding this comment.
Shouldn't be, it's O(2n) at worst with n == 1000 currently and runs every second. I don't really think the 2n case will happen either
src/stats.rs
Outdated
| @@ -68,6 +70,8 @@ pub struct PeerStats { | |||
| pub timestamp: Instant, | |||
| /// The last egress bandwidth estimate from the BWE subsystem, if enabled. | |||
| pub bwe_tx: Option<Bitrate>, | |||
| /// The egress loss over the last second expressed as a percentage value between 0.0 and 1.0. | |||
There was a problem hiding this comment.
or just multiply by 100 and make it an u8?
There was a problem hiding this comment.
Hmm, I think keeping it as f64 would be nice but I can reword it
There was a problem hiding this comment.
No preference, I just defaulted to f64. Do we have a stated preference for the project?
There was a problem hiding this comment.
Fair, f32 should be sufficient. Will change
There was a problem hiding this comment.
also if you say egress_loss_ratio it is clear that you return a ratio and I will expect numbers between 0 and 1
There was a problem hiding this comment.
also if you say egress_loss_ratio it is clear that you return a ratio and I will expect numbers between 0 and 1
Good point, although I know @algesten isn't a fan of long names :D
There was a problem hiding this comment.
Renamed and type changed
k0nserv
force-pushed
the
feature/egress-loss-stat
branch
from
0af7974
to
457c485
Compare
| now = now + Duration::from_millis(20); | ||
| let loss = reg | ||
| .loss(Duration::from_millis(150), now) | ||
| .expect("Should be able to calcualte loss"); |
|
lgtm |
Calculate egress loss based on TWCC reports.
k0nserv
force-pushed
the
feature/egress-loss-stat
branch
from
225b54c
to
e0058dc
Compare
Add an egress loss stat to
PeerStatsderived from TWCC reports.